Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Only vote on recent blocks #420

Merged
merged 6 commits into from
Jul 27, 2024
Merged

Only vote on recent blocks #420

merged 6 commits into from
Jul 27, 2024

Conversation

heifner
Copy link
Member

@heifner heifner commented Jul 26, 2024

Simplify vote restriction by only looking at block timestamp vs wall clock instead of flipping an enable_voting flag.
The testing_allow_voting change was needed because the tester creates blocks starting with 2020-01-01.

Resolves #418

heifner added 3 commits July 26, 2024 13:58
@heifner
GH-418 Simplify check on if create_and_send_vote_msg should be called
f3b8efb
@heifner
GH-418 Remove enable_immediate_voting, just going with block is recen…
e7f5f1f 
…t according to timestamp and wall clock
@heifner
GH-418 Tests create blocks that begin 2020-01-01. Rename allow_voting…
0c5f170 
… to testing_allow_voting to make it obvious it is only for testing and allow voting even if block is not recent.
@heifner heifner requested review from linh2931 and greg7mdp July 26, 2024 20:28
linh2931
linh2931 previously approved these changes Jul 26, 2024
View reviewed changes
@heifner heifner added the OCI Work exclusive to OCI team label Jul 26, 2024
@linh2931 linh2931 dismissed their stale review July 26, 2024 22:00

The default value of testing_allow_voting should be false.

@greg7mdp
Copy link
Contributor

Approving, but I'm a little uncomfortable that testing_allow_voting is used for two completely separate purposes:

  1. when producing a series of blocks in testing, vote on only the last 4 to save test time
  2. to disable the controller feature of not voting on old blocks, because tests create blocks with old timestamps

I think it would be cleaner to have two separate states, or even better removing the test optimization (first_usage), in which case testing_allow_voting could be renamed allow_voting_on_old_blocks.

@heifner
Copy link
Member Author

heifner commented Jul 27, 2024

  1. when producing a series of blocks in testing, vote on only the last 4 to save test time

If I remember correctly this made a big difference.

@heifner heifner merged commit 77dfedb into main Jul 27, 2024
36 checks passed
@heifner heifner deleted the GH-418-comment branch July 27, 2024 00:50
@ericpassmore
Copy link
Contributor

Note:start
group: STABILITY
category: INTERNALS
summary: Only vote on recent blocks and simply voting by comparing time.
Note:end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greg7mdp greg7mdp greg7mdp approved these changes

@linh2931 linh2931 linh2931 approved these changes

Assignees
No one assigned
Labels
OCI Work exclusive to OCI team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Do not vote on old blocks
4 participants
@heifner @greg7mdp @ericpassmore @linh2931